Skip to content

Fixes #38909 - Use syslog gem not in windows #927

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
sbernhard wants to merge 2 commits into
base: develop
Choose a base branch
from
Open

Fixes #38909 - Use syslog gem not in windows #927

sbernhard wants to merge 2 commits into from
+38 −1

Conversation

@sbernhard
Copy link
Contributor

@sbernhard sbernhard commented Nov 17, 2025

No description provided.

ekohl
ekohl reviewed Nov 17, 2025
View reviewed changes
Copy link
Member

@ekohl ekohl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you open a Redmine issue for this?

I'd also love to have some CI on GHA that uses a Windows runner to test if it works, but lack the specific knowledge (and time). Do you have any idea how hard that would be?

@sbernhard sbernhard changed the title Use syslog gem not in windows Fixes #38909 - Use syslog gem not in windows Nov 17, 2025
@sbernhard sbernhard force-pushed the syslog_not_for_win branch 16 times, most recently from 1b37d01 to 6f459ce Compare November 17, 2025 20:34
ekohl
ekohl reviewed Nov 17, 2025
View reviewed changes
@sbernhard sbernhard force-pushed the syslog_not_for_win branch 4 times, most recently from 0acc281 to cf25385 Compare November 17, 2025 21:05
@maximiliankolb
Copy link

maximiliankolb commented Nov 26, 2025 •
edited
Loading

@sbernhard Please create a Redmine issue & have a look at the failing tests. Please ping Ewoud after GHA are 🍏 for a final ACK.

edit: apologies; Redmine issue is open but GHA still fails. There must be another reason for that.

@sbernhard
Copy link
Contributor Author

sbernhard commented Nov 26, 2025

@sbernhard Please create a Redmine issue & have a look at the failing tests. Please ping Ewoud after GHA are 🍏 for a final ACK.

edit: apologies; Redmine issue is open but GHA still fails. There must be another reason for that.

everything done.

@ekohl
Copy link
Member

ekohl commented Dec 28, 2025

@sbernhard the Redmine check is still failing: the commit must reference the issue.

@sbernhard
Copy link
Contributor Author

sbernhard commented Dec 30, 2025

Fixed it. Thanks @ekohl

ekohl
ekohl reviewed Jan 1, 2026
View reviewed changes
Copy link
Member

@ekohl ekohl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at ruby/syslog@ffbfaa6 I wonder if it's really needed. I'd expect the gem (>= 0.2.0) to install on Windows and be a noop.

@sbernhard sbernhard force-pushed the syslog_not_for_win branch 2 times, most recently from 2aff80f to 91d8af2 Compare January 5, 2026 16:37
ekohl
ekohl reviewed Jan 5, 2026
View reviewed changes
Copy link
Member

@ekohl ekohl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By now this really evolved into just testing on Windows, which is still a good thing. Do you have any idea what is needed to actually make the tests work?

@ekohl
Copy link
Member

ekohl commented Jan 6, 2026 

C:/hostedtoolcache/windows/Ruby/3.3.10/x64/lib/ruby/3.3.0/bundled_gems.rb:69:in `require': cannot load such file -- libvirt (LoadError)

I think this (and the krb5) related tests should be skipped. The modules themselves (both dns_libvirt and dhcp_libvirt) both correctly guard behind load_classes so it should be possible.

The biggest risk I see is that we silently disable the tests without knowing it, but I'm sure we can find a way around that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ekohl ekohl ekohl left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

Waiting on contributor

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@sbernhard @maximiliankolb @ekohl